Repository navigation
Conversation
size-limit report 📦
|
fb54ebb to
fda7a91
Compare
fda7a91 to
1f9ac0a
Compare
1f9ac0a to
752d9d0
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 752d9d0. Configure here.
b565598 to
9ee53ad
Compare
ed04155 to
c78e44c
Compare
c78e44c to
81692f3
Compare
81692f3 to
33d45ee
Compare
7cbbd91 to
d24902e
Compare
| import { addNextjsServerSpanHooks, NEXTJS_SERVER_IGNORE_SPANS } from '../server/serverSpanHooks'; | ||
| import { nextjsUseCacheIntegration } from '../server/useCacheInstrumentation'; | ||
|
|
||
| export * from '@sentry/cloudflare'; |
There was a problem hiding this comment.
q: Does this one include an init? Might lead to some confusion.
There was a problem hiding this comment.
Cloudflare doesn't have init, so it should be good
| } | ||
| } | ||
|
|
||
| const nextjsIntegration = (): Integration => ({ |
There was a problem hiding this comment.
Can't tell if that is a good wow or bad wow :D
| ]; | ||
| const { tracesSampler } = options; | ||
| if (tracesSampler) { | ||
| // A `tracesSampler` can ignore the `parentSampled: false` that the Next.js integration sets for tunnel requests. |
There was a problem hiding this comment.
Might be good to actually have e2e coverage on the tunnelRoute feature (but this requires a new app)
| // on Node.js. A 404 page still names the root span. | ||
| const isErrorPageBehindMiddleware = | ||
| NEXTJS_ERROR_PAGE_ROUTES.includes(route) && rootSpanAttributes?.[SENTRY_SEGMENT_NAME_SOURCE] === 'route'; | ||
| // eslint-disable-next-line typescript/no-deprecated | ||
| const method = rootSpanAttributes?.[HTTP_REQUEST_METHOD] || rootSpanAttributes?.[HTTP_METHOD]; | ||
|
|
||
| // Only hoist the http.route attribute if the transaction doesn't already have it | ||
| if (!method || rootSpanAttributes?.[HTTP_ROUTE] || isErrorPageBehindMiddleware) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Bug: The isErrorPageBehindMiddleware check is too broad, causing it to incorrectly block error page route hoisting when a route handler, not middleware, has previously named the root span.
Severity: LOW
Suggested Fix
The condition should be more specific to only block hoisting when middleware has named the span. This could be achieved by setting a unique attribute on the root span within handleMiddlewareSpanStart that can be checked for, instead of relying on the shared SENTRY_SEGMENT_NAME_SOURCE === 'route' attribute.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/nextjs/src/server/serverSpanHooks.ts#L121-L130
Potential issue: In the Edge runtime, the condition to prevent hoisting an error page's
route to the root span is overly broad. The check `isErrorPageBehindMiddleware` uses
`SENTRY_SEGMENT_NAME_SOURCE === 'route'` to determine if middleware has already named
the span. However, route handlers can also set this attribute. If a route handler runs,
sets this attribute, and then an error occurs that renders an error page, this logic
will incorrectly prevent the error page's route (e.g., '/500') from being hoisted to the
transaction name. The transaction will incorrectly retain the name of the route handler
that failed, not the error page that was served.
`withSentry` from `@sentry/nextjs/cloudflare` wraps the Worker entry of a Next.js app on Cloudflare Workers, e.g. `.open-next/worker.js` of OpenNext or the fetch handler of vinext. It is `withSentry` of `@sentry/cloudflare` with the Next.js handling added: - It installs the OpenTelemetry async context strategy and context manager at module load, so the spans of Next.js nest and keep their OpenTelemetry context as on Node.js. - Its client gets the span hooks and `ignoreSpans` of the server `init`, the event processor for the control flow errors of React, the `use cache` integration, the Sentry propagator and the Next.js SDK metadata. - The propagator keeps the request span as parent when Next.js extracts an incoming trace that the root span already continued. Next.js does this when it misses its router server context, e.g. on Workers where `process.cwd()` is `/bundle`, and each continued request then had two segments. - The request spans of Next.js (`BaseServer.handleRequest`) are ignored, so the request span of `withSentry` is the only `http.server` span. The other Next.js spans become its children; it gets the route from their `next.route` and its status from the response. When the middleware answers the request or throws, the span is named `middleware GET`, like the middleware segment on Node.js. - Requests to the tunnel route are not sampled. - The server and edge `init` in `sentry.*.config.ts` create no client in the Worker. They hand over the build release of `withSentryConfig`, which only code that Next.js compiles can read. The `nextjs-16-cf-workers` e2e app now uses it. This runs the server tests that were skipped, and adds tests for D1 spans, the OpenTelemetry context and trace continuation. Its `compatibility_date` moves to 2026-02-19, the first date on which Next.js extracts the incoming trace again. A size-limit entry measures `withSentry` of the new entry with the build settings of wrangler. An import from `@sentry/node` or the server `init` module exceeds its limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0aa1a8f to
f930202
Compare
| // On Cloudflare Workers, the `http.server` span of `withSentry` from `@sentry/cloudflare` is the request root span. | ||
| if ( | ||
| attributes[ATTR_NEXT_SPAN_TYPE] !== 'BaseServer.handleRequest' && | ||
| attributes[SENTRY_ORIGIN] !== 'auto.http.cloudflare' | ||
| ) { |
There was a problem hiding this comment.
Bug: For middleware requests on Cloudflare, the root span's op is incorrectly set to 'http.server' instead of 'middleware' because the ATTR_NEXT_SPAN_NAME attribute is missing.
Severity: MEDIUM
Suggested Fix
In serverSpanHooks.ts, within the handleMiddlewareSpanStart function, set the ATTR_NEXT_SPAN_NAME attribute on the root span for the Cloudflare middleware execution path. This will align its behavior with the Node.js path, allowing the logic in enhanceHandleRequestRootSpan to correctly identify the request and set the transaction op to 'middleware'.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/nextjs/src/server/enhanceHandleRequestRootSpan.ts#L38-L42
Potential issue: For middleware-handled requests in a Cloudflare environment, the root
span's transaction `op` is incorrectly set to `'http.server'` instead of the expected
`'middleware'`. This occurs because the logic in `handleMiddlewareSpanStart` does not
set the `ATTR_NEXT_SPAN_NAME` attribute on the root span for Cloudflare. Consequently,
the logic in `enhanceHandleRequestRootSpan`, which relies on this attribute to override
the `op`, fails to identify it as a middleware request. This results in inconsistent
transaction classification between Node.js and Cloudflare deployments, potentially
affecting performance monitoring and filtering in Sentry.
Also affects:
packages/nextjs/src/server/serverSpanHooks.ts:158~168
(disclaimer, most of the additions are from
withSentry.test.tsand other tests)Adds
withSentryfrom@sentry/nextjs/cloudflarefor the Worker entry of a Next.js app on Cloudflare Workers, for example.open-next/worker.jsof OpenNext or the fetch handler of vinext. It iswithSentryof@sentry/cloudflarewith the Next.js handling of the serverinitadded, so a Next.js app on Workers gets the spans it gets on Node.js from one wrapper, without new options. This is the only new public API.Decisions:
@sentry/cloudflareloses the OpenTelemetry context of the Next.js spans, so their parents and context values break.BaseServer.handleRequest) are ignored, so the request span ofwithSentryis the onlyhttp.serverspan. The other Next.js spans become its children; it gets the route from theirnext.routeand its status from the response. When the middleware answers the request or throws, the span is namedmiddleware GET, like the middleware segment on Node.js.compatibility_date2026-02-19,process.cwd()is/bundleduring a request. Next.js then misses its router server context and extracts the incoming trace again. The propagator keeps the request span as parent in that case, else each continued request has two segments.sentry.server.config.tsandsentry.edge.config.tsstill run in the Worker but create no client there. Their options do not apply, and a debug log says so. On Workers they are optional: they only set theturbopacktag and hand over the build release ofwithSentryConfig, which only code that Next.js compiles can read. Without them, the release comes from thewithSentryoptions,SENTRY_RELEASEorCF_VERSION_METADATA. Apps keep them fornext dev, which runs on Node.js and does not use the Worker entry.instrumentation.tsstays required foronRequestError.Nextjsintegration.withSentryof@sentry/cloudflarecreates the client itself (once per isolate), and the options callback only returns options. Thesetupof an integration is the only place that gets this client before the request span starts, without a new option in@sentry/cloudflare. It registers the span hooks of the serverinit, the propagator, the tunnel route sampling and the event processor that drops the control flow errors of React and Next.js, so a Worker withoutinstrumentation.tsdrops them too. The serverinitchecks for this integration, so it does not add the processor to the global scope again.@sentry/cloudflarebecomes a dependency of@sentry/nextjs. Its dependencies are already in the dependency tree of@sentry/nextjs. It also aligns with other SDKs, such as@sentry/sveltekitKnown difference: on Workers, a release from the
withSentryoptions,SENTRY_RELEASEorCF_VERSION_METADATAwins over the build release. On Node.js, the build release wins over the environment.The
nextjs-16-cf-workerse2e app now uses it, which un-skips its server tests and adds tests for D1 spans, the OpenTelemetry context and trace continuation.🤖 Generated with Claude Code